-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/explicitly define help url for each rule #1138
Feat/explicitly define help url for each rule #1138
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1138 +/- ##
==========================================
- Coverage 97.02% 96.94% -0.09%
==========================================
Files 37 39 +2
Lines 4670 5035 +365
==========================================
+ Hits 4531 4881 +350
- Misses 139 154 +15 ☔ View full report in Codecov by Sentry. |
Still needs a bit of testing, but otherwise done :) |
This PR is ready for review :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I apologise for delay. I was conflicted with the topics because of the new ideas:
bhirsz/robotframework-cop-academy#33
I'm also refactoring defining the rules in new robocop heavily: https://github.com/bhirsz/robotframework-cop-academy/tree/feature/refactor_rules
That's why I was conflicted with introducing such breaking change, and then another. How do you feel about possibly postponing it, is it something you want sooner than later? Alternatively we can ignroe community rules for now (since they will be removed), and rename "DefaultRule" to "Rule" like in the beginning (sorry for the extra work). All the result may stay - in that case we will still have option to overwrite help url by creating own class but without any breaking change.
This is a non-breaking change ;) It extends the existing That being said, I'm fine with postponing this if you think you can find a better long-term solution :) |
You're right, I've taken a second look and I see it should be fine to merge. I will do it and make a new release. Thanks! |
075dee8
to
9018056
Compare
Closes #1123
This is a proof of concept for explicitly defining the help URL for each rule. This approach will require all default and community rules to be extended with a URL.Fixes rule help URLs for community rules and custom rules. URLs are now defined per rule instead of globally.
This is a backward compatible change.